Add an "ide" component - #13708
Conversation
|
Staged preview of the updated flutter.dev site (updated for commit acd7a64): https://flutter-dev-230821--www-pr13708-ide-component-djgv8rr8.web.app |
|
Staged preview of the updated docs.flutter.dev site (updated for commit acd7a64): https://flutter-docs-prod--docs-pr13708-ide-component-1g9w9f0b.web.app |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new interactive file-tree explorer component (IdeExplorer) for documentation sites, including its SASS styles, Jaspr Dart components, a custom markdown parser, and client-side interactivity scripts. The feedback highlights several critical issues: a potential hydration mismatch in SSR due to a global static counter, invalid HTML nesting of a inside a
| (reasoning, safety, and conciseness) using automated model judges | ||
| and expert human grading. | ||
|
|
||
| {% render "docs/ai/flutter_bench_task_explorer.md" %} |
There was a problem hiding this comment.
This is for demo purposes, and needs to be removed before merged
There was a problem hiding this comment.
This is super cool @ericwindmill! I haven't completed a full review, but thought I'd leave some initial comments. Feel free to push back on anything.
Thanks!
|
|
||
| /// Set up interactivity of the file/detail explorer created with | ||
| /// the `<IdeExplorer>` custom component. | ||
| void _setUpIdeExplorers() { |
There was a problem hiding this comment.
If it's possible, I'd prefer to avoid adding this much new imperative DOM logic. While Jaspr support for server components is still in-progress, we have a ComponentRef type that can be used for now to store and reference the rendered Markdown content.
Using that, the IdeExplorer might be able to become a client component that accepts Map<String, ComponentRef> instead of Map<String, Component> for customContents.
Worth trying, but if it ends up being too complicated or messy, don't worry about it. We can always revisit it once server components are finalized.
There was a problem hiding this comment.
If you end up keeping this imperative logic, review it and if needed, update it to ensure the aria states on the elements are updated as appropriate.
There was a problem hiding this comment.
I attempted this and it might just be over my head. I couldn't get past build failures after a few tries. I'm happy to try again with a little more guidance.
In general I agree it'd be great to remove the imperative logic
3 participants
Description of what this PR is changing or adding, and why:
This is adding an interactive IDE component, because its cool. I'm using it for the FlutterBench updates, but want to land this separately to keep the PR sane.
Depends on #13766